Skip to content

Proper TCP socket shutdown; Generic TCP timeout utils; built-in HTTP server timeouts; update docu #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 10 commits into from
Oct 8, 2024
Merged

Proper TCP socket shutdown; Generic TCP timeout utils; built-in HTTP server timeouts; update docu #34

merged 10 commits into from
Oct 8, 2024

Conversation

ivmarkov
Copy link
Collaborator

@ivmarkov ivmarkov commented Oct 8, 2024 •
edited
Loading

This PR implements:

  • A TcpShutdown trait with proper socket close and abort functionality for both STD and embassy-net. Needs testing for embassy-net
  • Generic TCP "timeout" decorators in edge_net which can be used by anyone
  • Built-in timeouts (both for each IO operation, as well as for the whole request-response) for the HTTP server

@ivmarkov ivmarkov changed the title Proper TCP socket shutdown; update docu Proper TCP socket shutdown; Generic TCP timeout utils; built-in HTTP server timeouts; update docu Oct 8, 2024
@ivmarkov ivmarkov mentioned this pull request Oct 8, 2024
Closed
AnthonyGrondin
AnthonyGrondin reviewed Oct 8, 2024
View reviewed changes
@AnthonyGrondin
Copy link
Contributor

I tested with esp-wifi + embassy-net and I can confirm that this fixes the issues I've been having with closing connections:

Here's a trace for a request issued with "Connection: Close" and following the redirection.

image

Here's a trace for a request issued with "Connection: Keep-Alive" and following the redirection.
image

@AnthonyGrondin
Copy link
Contributor

LGTM.
Thank you very much for taking care of this.

If some additional optional parameters are ever added to run(), it may become more user friendly to switch to a builder pattern, but at this point, this is more about UX than usability, and we should focus on usability for now.

@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Oct 8, 2024

If some additional optional parameters are ever added to run(), it may become more user friendly to switch to a builder pattern, but at this point, this is more about UX than usability, and we should focus on usability for now.

Agreed. A builder pattern can wait a bit.

@ivmarkov ivmarkov merged commit 6e017d9 into master Oct 8, 2024
1 check passed
@ivmarkov ivmarkov deleted the socket-shutdown branch November 3, 2024 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@AnthonyGrondin AnthonyGrondin AnthonyGrondin left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ivmarkov @AnthonyGrondin